QuickEdit: Add password field data to the pages quick edit #66567

louwie17 · 2024-10-29T14:42:58Z

What?

This adds a password field to the @wordpress/fields package for use within the DataForm component. In particular the page quick edit.
Related issue #64519.

Why?

The password field was missing from the page quick edit.

How?

A new password field is added to the @wordpress/fields component, which can be imported by using passwordField.
The field it-self is mimicked from the current implementation in the site editor sidebar:

gutenberg/packages/editor/src/components/post-status/index.js

Lines 222 to 263 in 2943dd3

    
           <VStack 
        
           	as="fieldset" 
        
           	spacing={ 4 } 
        
           	className="editor-change-status__password-fieldset" 
        
           > 
        
           	<CheckboxControl 
        
           		__nextHasNoMarginBottom 
        
           		label={ __( 
        
           			'Password protected' 
        
           		) } 
        
           		help={ __( 
        
           			'Only visible to those who know the password' 
        
           		) } 
        
           		checked={ showPassword } 
        
           		onChange={ 
        
           			handleTogglePassword 
        
           		} 
        
           	/> 
        
           	{ showPassword && ( 
        
           		<div className="editor-change-status__password-input"> 
        
           			<TextControl 
        
           				label={ __( 
        
           					'Password' 
        
           				) } 
        
           				onChange={ ( value ) => 
        
           					updatePost( { 
        
           						password: value, 
        
           					} ) 
        
           				} 
        
           				value={ password } 
        
           				placeholder={ __( 
        
           					'Use a secure password' 
        
           				) } 
        
           				type="text" 
        
           				id={ passwordInputId } 
        
           				__next40pxDefaultSize 
        
           				__nextHasNoMarginBottom 
        
           				maxLength={ 255 } 
        
           			/> 
        
           		</div> 
        
           	) } 
        
           </VStack>

Class names have just been updated.

The password field is displayed within the status dropdown as part of the combined fields API.

Testing Instructions

Enable the DataViews specific experiments under Gutenberg > Experiments
Make sure you have a block themed theme enabled.
Go to Appearance > Editor and click Pages
Switch the page view to the table view
Select a page and select the Details icon
A Status & Visibility field should now be displayed
Click on the visibility field and toggle the statuses, a password field should be shown if the status is not private
Add a password and update the page ( it should update correctly )
Select multiple pages, the Status & Visibility field should dissapear.

Testing Instructions for Keyboard

Screenshots or screencast

github-actions · 2024-10-29T14:43:15Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: louwie17 <[email protected]>
Co-authored-by: jameskoster <[email protected]>
Co-authored-by: oandregal <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

louwie17 · 2024-10-29T14:46:08Z

packages/fields/src/fields/password/edit.tsx

+			) }
+			{ hideLabelFromVision && (
+				<VisuallyHidden as="legend">{ label }</VisuallyHidden>
+			) }


I added a label for the top of the field, above the checkbox. This does cause a Password label to show twice when the checkbox is checked and the password field shows up.
Should we remove this and have it render without this label by default? ( cc: @jameskoster )

For now it probably makes sense to mimic what we have in the page inspector on trunk, IE no label, plus a horizontal rule between status / password fields:

Thanks for the PR!

Thanks @jameskoster, that sounds good, all done:

oandregal · 2024-10-30T17:37:38Z

packages/edit-site/src/components/post-edit/index.js

+				{
+					id: 'status_and_visibility',
+					label: __( 'Status & Visibility' ),
+					children: [ 'status', 'password' ].filter( ( child ) => {


The API for controlling visibility landed, but it didn't have support for combinedFields. We also have #66531 that aims to replace combinedFields API.

How would you like to move forward? Do we merge 66531 first and come back to this one later? Implement visibility for combinedFields as it is here? Any other approach?

Given this is ready, I would say merge this first, and replace the combinedFields usage as part of #66531
I also rebased this PR and made use of the isVisible function.

oandregal · 2024-10-30T17:39:55Z

packages/fields/src/fields/password/index.tsx

+import PasswordEdit from './edit';
+
+const passwordField: Field< BasePost > = {
+	id: 'password',


What's this field type? I presume text. We should make the type mandatory at some point.

Yes text would be the most appropriate. I added this as part of this commit: 0f1b2dd

Although given this is a password, I wonder if we should add support for a new type to manage hidden texts? Something like hidden-text.
While the password is just plain text in the actual field, I wonder if we should display it as hidden when shown in a table? ( with maybe a tooltip or show button to display it ).
cc: @jameskoster if this could be of benefit

That's a sensible thought. I think text is good for now because the field is not visible in any place other than QuickEdit.

oandregal · 2024-10-30T17:51:56Z

packages/fields/src/fields/password/edit.tsx

+	hideLabelFromVision,
+}: DataFormControlProps< BasePost > ) {
+	const { label } = field;
+	const [ showPassword, setShowPassword ] = useState( !! data.password );


In general, I think we should favor access to data via field.getValue( data ), otherwise we may be replicating the getValue logic in multiple places (not the case here, but I believe that should be a good practice for us). I was having a related conversation with @gigitux about passing field to the View component for the same reason.

Good call out, I replaced it to use getValue instead in this commit: 770c200

oandregal

I'm pre-approving this one, though there's this design feedback to address. It also needs rebasing from trunk.

I've also shared a question about a few moving pieces (but I'm fine shipping this first).

oandregal · 2024-10-31T10:10:09Z

packages/fields/src/fields/password/index.tsx

+	isVisible: ( item ) => item.status !== 'private',
+};
+
+export default passwordField;


Could you add a bit of JSDoc for this export, so it shows up in the docs (README) instead of "undocumented declaration"? (see parentField example)

Yup will do, one minute :)

Done: 6abc19e

oandregal · 2024-10-31T10:15:12Z

packages/fields/src/fields/password/edit.tsx

+	field,
+}: DataFormControlProps< BasePost > ) {
+	const [ showPassword, setShowPassword ] = useState(
+		!! field.getValue( { item: data } )


This is a nice detail, thanks.

oandregal · 2024-10-31T10:18:37Z

packages/edit-site/src/components/post-edit/style.scss

@@ -7,3 +7,10 @@
 		justify-content: center;
 	}
 }
+
+.dataforms-layouts-panel__field-dropdown {


Shouldn't these styles be part of the dataviews/fields package instead? There's probably more places where we're doing this wrong, but DataViews specific classes shouldn't be used by consumers. They are not public API and can change without notice.

I am not opposed to moving it to the password field specifically, there was two main reasons I end up doing it here:

Given the line is only relevant within the panel view in the status dropdown, it felled more like a higher level managed style.

I didn't want to make it the default for DataViews because of this previous conversation around combined fields

Having said that, I can add the same CSS to the password field specifically, so it is still panel specific.

mmm, I see your point. We need the combination of panel + field. These styles could live in edit-site (current), dataviews (but then would have wordpress field specific styles), or fields (but then would have dataviews-specific styles). None of the three options is ideal. Let's leave it as it is and keep an eye for similar things.

@louwie17 had a thought I wanted to share: what is this border for? is it for separating fields that are combined, like status & password are? If so, what if we refactor this to add the border automatically in that situation?

Co-authored-by: louwie17 <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: oandregal <[email protected]>

louwie17 commented Oct 29, 2024

View reviewed changes

louwie17 requested review from gigitux, youknowriad and oandregal October 29, 2024 14:46

louwie17 self-assigned this Oct 29, 2024

louwie17 added [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Experimental Experimental feature or API. labels Oct 29, 2024

louwie17 mentioned this pull request Oct 29, 2024

DataForm: replicate DocumentInspector in QuickEdit #64519

Open

11 tasks

louwie17 changed the title ~~[DataViews] Add password field data to the pages quick edit~~ QuickEdit: Add password field data to the pages quick edit Oct 29, 2024

oandregal reviewed Oct 30, 2024

View reviewed changes

oandregal approved these changes Oct 30, 2024

View reviewed changes

louwie17 added 14 commits October 31, 2024 10:15

Add password field

fc76d1b

Use password field in page data form

5dad068

Small change around hideLabelFromVision and styling fix

83bf28a

Stop relying on isValid for visibility

e132f95

Disable sorting and hiding so it doesn't show in DataViews

a4c0d85

Add translation

b21e81d

Remove old layout config for combined fields

34dcc07

Move bulk edit filtering up to useMemo

1a5147a

Update import

5d1234e

Update class name

c969697

Update to use new isVisible function

67fa0b8

Remove duplicate label in password field and add type

0f1b2dd

Make use of getValue instead of directly referencing the data object

770c200

Add border for password field in dataforms combined edit.

02b8eb0

louwie17 force-pushed the add/password_field_data_forms branch from f896b7b to 02b8eb0 Compare October 31, 2024 09:57

oandregal reviewed Oct 31, 2024

View reviewed changes

Add JSDoc for password field export and update README

6abc19e

oandregal reviewed Oct 31, 2024

View reviewed changes

oandregal enabled auto-merge (squash) October 31, 2024 10:34

oandregal merged commit fc2bf3a into WordPress:trunk Oct 31, 2024
65 checks passed

github-actions bot added this to the Gutenberg 19.7 milestone Oct 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QuickEdit: Add password field data to the pages quick edit #66567

QuickEdit: Add password field data to the pages quick edit #66567

louwie17 commented Oct 29, 2024 •

edited by oandregal

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

louwie17 Oct 29, 2024

jameskoster Oct 30, 2024

louwie17 Oct 31, 2024

oandregal Oct 30, 2024

louwie17 Oct 31, 2024

oandregal Oct 30, 2024

louwie17 Oct 31, 2024

louwie17 Oct 31, 2024

oandregal Oct 31, 2024

oandregal Oct 30, 2024

louwie17 Oct 31, 2024

oandregal left a comment •

edited

Loading

oandregal Oct 31, 2024

louwie17 Oct 31, 2024

louwie17 Oct 31, 2024

oandregal Oct 31, 2024

oandregal Oct 31, 2024 •

edited

Loading

louwie17 Oct 31, 2024

oandregal Oct 31, 2024

oandregal Oct 31, 2024

	<VStack
	as="fieldset"
	spacing={ 4 }
	className="editor-change-status__password-fieldset"
	>
	<CheckboxControl
	__nextHasNoMarginBottom
	label={ __(
	'Password protected'
	) }
	help={ __(
	'Only visible to those who know the password'
	) }
	checked={ showPassword }
	onChange={
	handleTogglePassword
	}
	/>
	{ showPassword && (
	<div className="editor-change-status__password-input">
	<TextControl
	label={ __(
	'Password'
	) }
	onChange={ ( value ) =>
	updatePost( {
	password: value,
	} )
	}
	value={ password }
	placeholder={ __(
	'Use a secure password'
	) }
	type="text"
	id={ passwordInputId }
	__next40pxDefaultSize
	__nextHasNoMarginBottom
	maxLength={ 255 }
	/>
	</div>
	) }
	</VStack>

QuickEdit: Add password field data to the pages quick edit #66567

QuickEdit: Add password field data to the pages quick edit #66567

Conversation

louwie17 commented Oct 29, 2024 • edited by oandregal Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

github-actions bot commented Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

louwie17 commented Oct 29, 2024 •

edited by oandregal

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

oandregal left a comment •

edited

Loading

oandregal Oct 31, 2024 •

edited

Loading